*: hide secure information in SHOW PROCESSLIST #4451
Conversation
ast/ast.go
Outdated
| @@ -37,6 +37,8 @@ type Node interface { | |||
| Text() string | |||
| // SetText sets original text to the Node. | |||
| SetText(text string) | |||
| // SecureText is different from Text that it hide password information. | |||
| SecureText() string | |||
There was a problem hiding this comment.
I think this method should not be put in Node interface.
How about defining a CrusialStmt interface which only has a SecureText method?
There was a problem hiding this comment.
So we implement SecureText() for so many
*ast.CreateUserStmt, *ast.DropUserStmt, *ast.AlterUserStmt, *ast.SetPwdStmt, *ast.GrantStmt,
*ast.RevokeStmt, *ast.AlterTableStmt, *ast.CreateDatabaseStmt, *ast.CreateIndexStmt, *ast.CreateTableStmt,
*ast.DropDatabaseStmt, *ast.DropIndexStmt, *ast.DropTableStmt, *ast.RenameTableStmt, *ast.TruncateTableStmt:
and most of those implemention is just ... ?
func (x *XXX) SecureText() string {
return x.Text()
}
There was a problem hiding this comment.
Well, not all crucial statement need to have secure text.
Then how about define a SensitiveStatement which has SecureText?
There was a problem hiding this comment.
if ss, ok := node.(SensitiveStmt); ok {
text = ss.SecureText()
} else {
text = node.Text()
}
There was a problem hiding this comment.
Could we add SecureText to StmtNode interface?
There was a problem hiding this comment.
@shenli
SecureText only needed by 4 statements, I think adding it to all StmtNode is not worthy.
|
@tiancaiamao Any update? |
ast/misc.go
Outdated
| @@ -406,6 +408,11 @@ type SetPwdStmt struct { | |||
| Password string | |||
| } | |||
|
|
|||
| // SecureString implements SensitiveStatement interface. | |||
| func (n *SetPwdStmt) SecureString() string { | |||
|
Please add test. |
|
LGTM |
|
/run-all-test |
|
LGTM |
show processlistshould not display something like password.@zimulala @coocood @shenli